fix(consume): fix `consume_direct.sh` by quoting the testid passed to `--filter` #1987

felix314159 · 2025-08-04T15:39:14Z

🗒️ Description

Title says it all, simple fix. Context: #1843 , more specifically this comment by flcl

What is fixed?

Before this PR, if you run sth like
uv run consume direct --bin=nethtest --input=stable@latest --traces --dump-dir=evm-tmp -n 8 -k push0
and then try to run a consume_direct.sh file like
./evm-tmp/push0_contracts/tests-shanghai-eip3855_push0-test_push0.py::test_push0_contracts[fork_Shanghai-blockchain_test_from_state_test-gas_cost]/consume_direct.sh
you will see sth like:

Unhandled exception: System.Text.RegularExpressions.RegexParseException: Invalid pattern '^(tests/shanghai/eip3855_push0/test_push0.py::test_push0_contracts[fork_Shanghai-blockchain_test_from_state_test-gas_cost])' at offset 82. [x-y] range in reverse order.
   at System.Text.RegularExpressions.RegexParser.ScanCharClass(Boolean caseInsensitive, Boolean scanOnly)
   at System.Text.RegularExpressions.RegexParser.ScanRegex()
   at System.Text.RegularExpressions.RegexParser.Parse(String pattern, RegexOptions options, CultureInfo culture)
   at System.Text.RegularExpressions.Regex..ctor(String pattern, CultureInfo culture)
   at System.Text.RegularExpressions.RegexCache.GetOrAdd(String pattern)
   at System.Text.RegularExpressions.Regex.Match(String input, String pattern)
   at Nethermind.Test.Runner.BlockchainTestsRunner.RunTestsAsync() in /home/user/Documents/nethermind/src/Nethermind/Nethermind.Test.Runner/BlockchainTestsRunner.cs:line 35
   at Nethermind.Test.Runner.Program.RunBlockTest(String path, Func`2 testRunnerBuilder) in /home/user/Documents/nethermind/src/Nethermind/Nethermind.Test.Runner/Program.cs:line 115
   at Nethermind.Test.Runner.Program.Run(ParseResult parseResult, CancellationToken cancellationToken) in /home/user/Documents/nethermind/src/Nethermind/Nethermind.Test.Runner/Program.cs:line 90
   at System.CommandLine.Invocation.InvocationPipeline.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)

This PR fixes this, after applying it the command works as expected (e.g.):

tests/shanghai/eip3855_push0/test_push0.py::test_push0_contracts[fork_Shanghai-blockchain_test_from_state_test-gas_cost] PASS

Both Nethtest and geth's evm were leading to faulty sh files and this PR fixes both.

🔗 Related Issues or PRs

N/A.

✅ Checklist

All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
```
uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
```
All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
All: Considered adding an entry to CHANGELOG.md.
All: Considered updating the online docs in the ./docs/ directory.
All: Set appropriate labels for the changes (only maintainers can apply labels).
Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.
Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

…_direct.sh would be invalid because the value after the --filter flag was not wrapped in parenthesis

marioevz

Suggestion on a change to the approach, thanks!

src/ethereum_clis/clis/nethermind.py

felix314159 · 2025-08-05T14:00:07Z

I have now figured out what we must change to also fix geth's evm command in the sh files. There actually are 2 issues with blockchain tests:

value after --run flag has to be put in quotations
last part of the command needs to have \ escapes added before brackets [ and ]

So e.g. currently in the .sh file you would see sth like:

#!/bin/bash

/home/user/Documents/go-ethereum/build/bin/evm --verbosity 100 blocktest --trace --run tests/shanghai/eip3855_push0/test_push0\.py::test_push0_contracts\[fork_Shanghai\-blockchain_test_from_state_test\-before_jumpdest\] evm-tmp/push0_contracts/tests-shanghai-eip3855_push0-test_push0.py::test_push0_contracts[fork_Shanghai-blockchain_test_from_state_test-before_jumpdest]/fixtures.json

when actually the command should be

/home/user/Documents/go-ethereum/build/bin/evm --verbosity 100 blocktest --trace --run 'tests/shanghai/eip3855_push0/test_push0\.py::test_push0_contracts\[fork_Shanghai\-blockchain_test_from_state_test\-before_jumpdest\]' evm-tmp/push0_contracts/tests-shanghai-eip3855_push0-test_push0.py::test_push0_contracts\[fork_Shanghai-blockchain_test_from_state_test-before_jumpdest\]/fixtures.json

felix314159 · 2025-08-05T14:25:36Z

Can be merged as is, I tested both state test and blockchain test sh files created via both nethtest and geth's evm and it all works now

felix314159 · 2025-08-07T14:36:33Z

Is making use of shlex a requirement to get this merged? The current solution works well and has been tested, I don't think this is a part of the codebase we will touch often

danceratopz · 2025-08-14T09:05:40Z

I'd like to help you get this over the finishline @felix314159!

I'm thinking something along these lines...

def quote_arg(arg: str, shell: Shell | None = None) -> str:
    shell = shell or _detect_default_shell()
    if shell == "posix":
        return shlex.quote(arg)
    elif shell == "cmd":
        # Use Windows C-runtime rules; safest is to let Python do it.
        return subprocess.list2cmdline([arg])
    else:
        raise Exception(f"oops unsupported shell {shell}")

I'd prefer to use standard libraries as Mario suggests, to avoid surprises with other special characters in the future.

felix314159 · 2025-08-14T09:22:29Z

Let's not handle windows to keep things simple. I reworked the PR with shlex and it still works and i must admit it is easier to read now than before. Can be merged now

danceratopz

Thanks @felix314159 - LGTM!

I tested geth quickly, but not Nethermind.

fixed issue reported by flcl42 where the command contained in consume…

8eb44ff

…_direct.sh would be invalid because the value after the --filter flag was not wrapped in parenthesis

felix314159 requested review from marioevz and spencer-tb August 4, 2025 15:39

marioevz reviewed Aug 4, 2025

View reviewed changes

src/ethereum_clis/clis/nethermind.py Outdated Show resolved Hide resolved

src/ethereum_clis/clis/nethermind.py Outdated Show resolved Hide resolved

fixed sh command for geths evm, revised nethtest fix

4d71320

felix314159 requested a review from marioevz August 5, 2025 14:26

removed logging workaround

e767852

felix314159 added the P-low label Aug 11, 2025

use shlex

c0d1826

danceratopz changed the title ~~fix(consume): consume_direct.sh files contained invalid commands because value after --filter flag was not enclosed in parenthesis (issue reported by flcl42)~~ fix(consume): fix consume_direct.sh by quoting the testid passed to --filter Aug 14, 2025

danceratopz added type:bug Something isn't working scope:consume Scope: Consume command suite labels Aug 14, 2025

docs: update changelog

5c9316f

danceratopz approved these changes Aug 14, 2025

View reviewed changes

danceratopz merged commit 5220d21 into ethereum:main Aug 14, 2025
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(consume): fix `consume_direct.sh` by quoting the testid passed to `--filter` #1987

fix(consume): fix `consume_direct.sh` by quoting the testid passed to `--filter` #1987

felix314159 commented Aug 4, 2025 •

edited

Loading

Uh oh!

marioevz left a comment

Uh oh!

Uh oh!

Uh oh!

felix314159 commented Aug 5, 2025 •

edited

Loading

Uh oh!

felix314159 commented Aug 5, 2025 •

edited

Loading

Uh oh!

felix314159 commented Aug 7, 2025

Uh oh!

danceratopz commented Aug 14, 2025

Uh oh!

felix314159 commented Aug 14, 2025

Uh oh!

danceratopz left a comment

Uh oh!

Uh oh!

Uh oh!

fix(consume): fix consume_direct.sh by quoting the testid passed to --filter #1987

fix(consume): fix consume_direct.sh by quoting the testid passed to --filter #1987

Conversation

felix314159 commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗒️ Description

What is fixed?

🔗 Related Issues or PRs

✅ Checklist

Uh oh!

marioevz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

felix314159 commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felix314159 commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felix314159 commented Aug 7, 2025

Uh oh!

danceratopz commented Aug 14, 2025

Uh oh!

felix314159 commented Aug 14, 2025

Uh oh!

danceratopz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fix(consume): fix `consume_direct.sh` by quoting the testid passed to `--filter` #1987

fix(consume): fix `consume_direct.sh` by quoting the testid passed to `--filter` #1987

felix314159 commented Aug 4, 2025 •

edited

Loading

felix314159 commented Aug 5, 2025 •

edited

Loading

felix314159 commented Aug 5, 2025 •

edited

Loading